Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

doc: change error message testing policy #31421

Conversation

tniessen
Copy link
Member

@tniessen tniessen commented Jan 20, 2020

Dynamic error messages often contain important information that depends on the context, e.g., which argument caused an ERR_INVALID_ARG_TYPE, which type was expected, and which type was received. I don't think that internal breakage when changing a dynamic error message should prevent us from testing such properties properly. (Not to test the error message, but rather the API that uses the error code.)

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • documentation is changed or added
  • commit message follows commit guidelines

Dynamic error messages often contain important information that
depends on the context, e.g., which argument caused an
ERR_INVALID_ARG_TYPE, which type was expected, and which type was
received. I don't think that internal breakage when changing a dynamic
error message should prevent us from testing such properties properly.
@nodejs-github-bot nodejs-github-bot added the doc Issues and PRs related to the documentations. label Jan 20, 2020
@tniessen tniessen added the meta Issues and PRs related to the general management of the project. label Jan 20, 2020
Copy link
Member

@BridgeAR BridgeAR left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This whole section seems somewhat outdated. We do not check for any new errors in test/parallel/test-internal-errors.js. Instead, those are tested by checking the actual codes error condition. The basic functionality is tested internally with special test errors. I suggest to replace this section therefore with a note that new error codes should be tested by code that triggers those.

Copy link
Member

@Trott Trott left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's also validating-minimally-with-a-regexp vs. validating-the-entire-text.

I'm happy with this material removed entirely, updated as you have done here, or updated as @BridgeAR suggests.

tniessen added a commit that referenced this pull request Jan 29, 2020
Dynamic error messages often contain important information that
depends on the context, e.g., which argument caused an
ERR_INVALID_ARG_TYPE, which type was expected, and which type was
received. I don't think that internal breakage when changing a dynamic
error message should prevent us from testing such properties properly.

PR-URL: #31421
Reviewed-By: Rich Trott <rtrott@gmail.com>
@tniessen
Copy link
Member Author

Landed in d65e6a5. I agree that larger changes are necessary.

@tniessen tniessen closed this Jan 29, 2020
codebytere pushed a commit that referenced this pull request Feb 17, 2020
Dynamic error messages often contain important information that
depends on the context, e.g., which argument caused an
ERR_INVALID_ARG_TYPE, which type was expected, and which type was
received. I don't think that internal breakage when changing a dynamic
error message should prevent us from testing such properties properly.

PR-URL: #31421
Reviewed-By: Rich Trott <rtrott@gmail.com>
@codebytere codebytere mentioned this pull request Feb 17, 2020
codebytere pushed a commit that referenced this pull request Mar 15, 2020
Dynamic error messages often contain important information that
depends on the context, e.g., which argument caused an
ERR_INVALID_ARG_TYPE, which type was expected, and which type was
received. I don't think that internal breakage when changing a dynamic
error message should prevent us from testing such properties properly.

PR-URL: #31421
Reviewed-By: Rich Trott <rtrott@gmail.com>
codebytere pushed a commit that referenced this pull request Mar 17, 2020
Dynamic error messages often contain important information that
depends on the context, e.g., which argument caused an
ERR_INVALID_ARG_TYPE, which type was expected, and which type was
received. I don't think that internal breakage when changing a dynamic
error message should prevent us from testing such properties properly.

PR-URL: #31421
Reviewed-By: Rich Trott <rtrott@gmail.com>
@codebytere codebytere mentioned this pull request Mar 17, 2020
codebytere pushed a commit that referenced this pull request Mar 30, 2020
Dynamic error messages often contain important information that
depends on the context, e.g., which argument caused an
ERR_INVALID_ARG_TYPE, which type was expected, and which type was
received. I don't think that internal breakage when changing a dynamic
error message should prevent us from testing such properties properly.

PR-URL: #31421
Reviewed-By: Rich Trott <rtrott@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
doc Issues and PRs related to the documentations. meta Issues and PRs related to the general management of the project.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants